Skip to content

🚸 Improve QDMI Integration#1694

Merged
burgholzer merged 17 commits into
mainfrom
smoother-qdmi-integration
May 12, 2026
Merged

🚸 Improve QDMI Integration#1694
burgholzer merged 17 commits into
mainfrom
smoother-qdmi-integration

Conversation

@burgholzer
Copy link
Copy Markdown
Member

@burgholzer burgholzer commented May 11, 2026

Description

This PR originally only aimed to address #1352.
However, working on it prompted several follow-ups and improvements for the QDMI support layer as well as a couple of bugfixes.

The main change here is still that all QDMI devices are now built as shared libraries by default and that the respective dynamic wrappers have been removed.
The QDMI Qiskit wrapper now supports multi-controlled gates as exposed by the DDSIM device.
The QDMI Python tests have been significantly streamlined, and a lot of the mocking code is replaced with actual device executions.

I will leave this in draft until CI is green because I expect Windows to cause some problems.
Edit: As expected, Windows is acting up big again.
Edit2: I think I have a working version now. Only took 10h 🫠

Fixes #1352

Checklist

  • The pull request only contains commits that are focused and relevant to this change.
  • I have added appropriate tests that cover the new/changed functionality.
  • I have updated the documentation to reflect these changes.
  • I have added entries to the changelog for any noteworthy additions, changes, fixes, or removals.
  • I have added migration instructions to the upgrade guide (if needed).
  • The changes follow the project's style guidelines and introduce no new warnings.
  • The changes are fully tested and pass the CI checks.
  • I have reviewed my own code changes.

If PR contains AI-assisted content:

  • I have disclosed the use of AI tools in the PR description as per our AI Usage Guidelines.
  • AI-assisted commits include an Assisted-by: [Model Name] via [Tool Name] footer.
  • I confirm that I have personally reviewed and understood all AI-generated content, and accept full responsibility for it.

@burgholzer burgholzer added this to the QDMI Support milestone May 11, 2026
@burgholzer burgholzer self-assigned this May 11, 2026
@burgholzer burgholzer added refactor Anything related to code refactoring minor Minor version update QDMI Anything related to QDMI backport-potential Changes to be backported to the stable branch labels May 11, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 11, 2026

Codecov Report

❌ Patch coverage is 92.59259% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/qdmi/driver/Driver.cpp 50.0% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@burgholzer burgholzer force-pushed the smoother-qdmi-integration branch from 14740d4 to 5e96b16 Compare May 11, 2026 19:22
@burgholzer burgholzer marked this pull request as ready for review May 11, 2026 19:42
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

Release Notes

  • New Features

    • Added support for multi-controlled gates (mcx, mcz, mcrx, etc.) in the QDMI Qiskit backend converter.
    • Added measurement operation to the SC QDMI device.
  • Changed

    • QDMI devices now built as shared libraries by default.
    • Minimum Qiskit version requirement raised to 1.1.0.
    • Requires LLVM 22.1 for C++ library builds and builds MLIR by default.
    • Removed dynamic device wrapper variants; use builtin devices directly.
  • Bug Fixes

    • Fixed DD sampler segmentation fault when idle classical bits are present.

Walkthrough

Builds QDMI device libraries as shared objects, removes dynamic wrapper sources and static-device declarations, switches the driver to runtime-load device libraries (Windows-aware), extends Qiskit backend mapping for multi-controlled gates, fixes DD sampling with measurements, and reorganizes tests, docs, and packaging.

Changes

QDMI Architecture Refactor

Layer / File(s) Summary
CMake helper and test macros
cmake/AddMQTCoreLibrary.cmake, cmake/PackageAddTest.cmake
add_mqt_core_library gains FORCE_SHARED/HIDDEN_VISIBILITY; test macros set BUILD_WITH_INSTALL_RPATH = FALSE.
Device library build changes
src/qdmi/devices/dd/CMakeLists.txt, src/qdmi/devices/na/CMakeLists.txt, src/qdmi/devices/sc/CMakeLists.txt, pyproject.toml
Device CMakeLists invoke add_mqt_core_library(... FORCE_SHARED HIDDEN_VISIBILITY ...), remove auxiliary _dyn targets/aliases, and update packaging targets.
Remove dynamic wrapper sources
src/qdmi/devices/na/DynDevice.cpp, src/qdmi/devices/sc/DynDevice.cpp
Deleted C-style _DYN_QDMI_device_* wrapper implementations that forwarded to builtin device APIs.
Remove static-device declarations
include/mqt-core/qdmi/driver/Driver.hpp
Removed DECLARE_STATIC_LIBRARY macro usage and generated static-device class declarations.
Driver runtime loading & CMake wiring
src/qdmi/driver/Driver.cpp, src/qdmi/driver/CMakeLists.txt
Switch driver to dynamic-load device libs, add DYN_DEV_LIBS compile-time list, platform rpath/DLL-copy handling, and adjust link interface and dependencies.
Device configuration
json/sc/device.json
Add measure operation to SC device JSON.

Qiskit Backend Multi-Controlled Gate Support

Layer / File(s) Summary
Gate mapping enhancement
python/mqt/core/plugins/qiskit/backend.py
Augment Qiskit canonical mappings with mcx/mcphase/mcp, expand alias maps (e.g., cu/cu3), normalize device op names, and treat class-based vs instance-based gates when populating Qiskit Target.
Device ops update
src/qdmi/devices/dd/Device.cpp
Removed mcz, mcrx, mcry, mcrz entries from the DD device OPERATIONS registry (handled via backend mapping).

Bug Fixes & Modernization

Layer / File(s) Summary
DD simulation fix
src/dd/Simulation.cpp
Skip permutation/garbage reduction for circuits with measurements; use runtime permutation for measured qubit indexing.
IR modernization
src/ir/QuantumComputation.cpp
Use std::views::values when scanning register maps in getQubitRegister.

Test Infrastructure Refactor

Layer / File(s) Summary
Central fixture removal
test/python/plugins/qiskit/conftest.py
Removed centralized MockQDMIDevice and many shared fixtures.
Localized mock backend tests
test/python/plugins/qiskit/test_mock_backend.py
Add in-file MockQDMIDevice, mock_qdmi_device_factory, QASM parsing and conversion/mapping tests previously in removed modules.
Backend tests using real device
test/python/plugins/qiskit/test_backend.py
Add ddsimb_backend fixture that discovers DDSIM via fomac.Session(); refactor many tests to run against real DDSIM and expand parameter/binding/result query coverage.
Estimator & Sampler tests
test/python/plugins/qiskit/test_estimator.py, test/python/plugins/qiskit/test_sampler.py
Introduce estimator/sampler fixtures that locate DDSIM and update result-data access patterns to pub_result.data.
Converter tests moved/removed
test/python/plugins/qiskit/test_converters.py
Old converter test module removed; coverage moved into test_mock_backend.py.
Provider warning cleanup
test/python/plugins/qiskit/test_provider.py
Removed module-level and per-test warning filters.
C++ test wiring updates
test/qdmi/*, test/fomac/CMakeLists.txt, test/na/fomac/CMakeLists.txt, test/qdmi/driver/*
Test CMakeLists updated to reference device targets and copy DLLs on Windows; driver test DYN_DEV_LIBS updated; removed dynamic-device test parameterizations and suites relying on _dyn targets.
Driver test adjustments
test/qdmi/driver/test_driver.cpp
Removed SetUpTestSuite dynamic loader and dynamic-device presence test.

Docs & Packaging

Layer / File(s) Summary
CHANGELOG & UPGRADING
CHANGELOG.md, UPGRADING.md
Document added/changed/removed items: measurement instruction for SC device, multi-controlled gates support, shared-library builds for builtin devices, removal of wrapper dyn devices, Qiskit minimum bumped to 1.1.0, C++ build LLVM/MLIR notes, and DD sample fix.
pyproject and packaging
pyproject.toml
Bump qiskit[qasm3-import] minimum to >=1.1.0; remove _dyn build targets from scikit-build targets.
Driver docs
docs/qdmi/driver.md
Reword objective to reference multiple preloaded devices instead of a single statically linked NA device.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

c++, python

Suggested reviewers

  • ystade

"🐰
I hopped through CMake fields today,
swapped wrappers for shared-stack clay,
the driver seeks libs at runtime's door,
multi-control gates bound once more,
tests now sniff the wild — hooray!"

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch smoother-qdmi-integration

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
test/python/plugins/qiskit/test_sampler.py (1)

56-58: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Stale comment: this is no longer a mock backend.

This test now executes against a real DDSIM device, so the "mock backend returns random distribution" comment is misleading. With DDSIM, the result of |00⟩ + |11⟩ measured in the computational basis should only ever yield "00" or "11" — you can additionally assert that the distribution is restricted to those two outcomes to make the test more meaningful now that results are deterministic.

📝 Suggested fix
-    # Check that we got some counts (mock backend returns random distribution)
-    counts = bit_array.get_counts()
-    assert sum(counts.values()) == 100
+    # Bell state: only "00" and "11" outcomes should appear
+    counts = bit_array.get_counts()
+    assert sum(counts.values()) == 100
+    assert set(counts).issubset({"00", "11"})
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/python/plugins/qiskit/test_sampler.py` around lines 56 - 58, The comment
is stale because the test now runs on a real DDSIM backend; update the test
around bit_array.get_counts() by removing the "mock backend" phrasing and add an
assertion that the observed outcome keys are only "00" and "11" (e.g., assert
set(counts.keys()) <= {"00","11"}) in addition to the existing total-count check
so the test verifies the expected deterministic Bell-state outcomes; locate this
change in the test function using bit_array.get_counts() and the counts
variable.
test/python/plugins/qiskit/test_estimator.py (1)

36-51: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Stale "mock backend" comment; tighten the assertion now that DDSIM is deterministic.

The comment on line 47 is no longer accurate — DDSIM is a real simulator, not a mock returning random values. For the simple |0⟩ state, ⟨Z⟩ should be 1.0 and ⟨X⟩ should be 0.0, and the test calls run(..., precision=None). You can additionally assert the expected values (with pytest.approx to account for sampling noise) to actually validate behavior rather than only shape.

📝 Suggested fix
-    # Values are simulated by mock backend (random), so we just check structure
-    evs = pub_result.data["evs"]
+    # For |0>: <Z> = 1.0 and <X> = 0.0
+    evs = pub_result.data["evs"]
     stds = pub_result.data["stds"]
     assert evs.shape == (2,)  # 2 observables
     assert stds.shape == (2,)
+    assert evs[0] == pytest.approx(1.0, abs=0.1)
+    assert evs[1] == pytest.approx(0.0, abs=0.1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/python/plugins/qiskit/test_estimator.py` around lines 36 - 51, The
test_estimator_run_simple_observable has a stale comment about a "mock backend"
and only asserts shapes; update it to assert the actual expected expectation
values from the deterministic DDSIM run: remove or update the comment and add
assertions on pub_result.data["evs"] using pytest.approx to check evs[0] ==
approx(1.0) and evs[1] == approx(0.0) (and keep the shape asserts if desired);
reference the test function test_estimator_run_simple_observable, the
estimator.run(...) call and the pub_result.data["evs"] / ["stds"] fields when
making the assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/qdmi/driver/CMakeLists.txt`:
- Around line 55-60: The rpath entries are currently set with
target_link_options(... INTERFACE ...), which only propagates to consumers and
not to the driver's own binary (so dlopen() inside the driver won't find device
libs); replace that with set_target_properties(${TARGET_NAME} PROPERTIES
BUILD_RPATH "<list>" (or INSTALL_RPATH if you want it for install) using the
same generator expressions $<TARGET_FILE_DIR:MQT::CoreQDMINaDevice>,
$<TARGET_FILE_DIR:MQT::CoreQDMIScDevice>,
$<TARGET_FILE_DIR:MQT::CoreQDMI_DDSIM_Device> so the rpath is embedded into the
driver itself; remove the INTERFACE target_link_options entries or keep only
non-INTERFACE options if needed.

In `@src/qdmi/driver/Driver.cpp`:
- Around line 44-81: Add Doxygen-style comments for the two Windows loader
helpers so their behavior is clear; above getDriverDirectory() document what it
does (returns the directory of the current module), note it is [[nodiscard]],
describe failure behavior (returns empty path), and summarize the Windows APIs
used (GetModuleHandleExW/GetModuleFileNameW) and why a loop/resizing is
necessary; above loadDeviceLibrary(const std::string& libName) document the
parameter (libName), explain path resolution (if libName has a parent path use
it, otherwise resolve relative to getDriverDirectory()), describe the return
value (HMODULE or null on failure), and list that it uses LoadLibraryExW with
LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS; include
`@param`, `@return` and brief one-line description for each function following the
project Doxygen style.
- Around line 383-386: Driver::Driver() currently calls addDynamicDeviceLibrary
for each entry in DYN_DEV_LIBS and allows any dlopen/LoadLibrary exception to
bubble out (breaking Driver::get() and C entrypoints). Wrap each
addDynamicDeviceLibrary(lib, prefix) invocation in a try/catch that catches
std::exception (and a catch(...) as fallback), log a clear error including the
failing lib/prefix and the exception.what() (or a generic message for non-std
exceptions), and continue to the next library so missing builtin DLLs are
skipped instead of aborting startup.

In `@test/python/plugins/qiskit/test_backend.py`:
- Around line 68-92: Add Google-style docstrings to the four helper functions
_single_qubit_circuit, _two_qubit_circuit, _two_qubit_barrier_circuit, and
_two_qubit_circuit_without_measurements: for each function include a one-line
summary, a short description of what the circuit contains, a Args section (none
or describe omitted params if any), and a Returns section specifying that a
QuantumCircuit is returned; place the docstring immediately below the def line
in triple quotes following Google-style formatting.

In `@test/python/plugins/qiskit/test_estimator.py`:
- Around line 142-143: Update the stale test comment in test_estimator.py:
replace "mock random results" with text stating the test runs against the DDSIM
simulator (deterministic up to sampling noise) and mention that for the
Bell-state circuit the expected expectation values are known (II=1, XX=1, YY=-1,
ZZ=1, IX=0, YI=0) and can be asserted with pytest.approx; optionally add those
pytest.approx assertions against the estimator outputs to make the test
deterministic instead of relying on a no-crash check.
- Around line 24-33: The estimator fixture duplicates DDSIM device discovery
logic found in test_sampler.py; extract that discovery into a shared fixture
named ddsim_backend in test/python/plugins/qiskit/conftest.py (use the same
logic that creates QDMIBackend(device=device, provider=None) from fomac.Session
devices) and then change def estimator() to def estimator(ddsim_backend) ->
QDMIEstimator: return QDMIEstimator(ddsim_backend), replacing the repeated
fomac.Session/QDMIBackend code; reference the existing estimator fixture,
QDMIEstimator, QDMIBackend, and the sampler fixture in test_sampler.py when
implementing the shared ddsim_backend fixture.

In `@test/python/plugins/qiskit/test_mock_backend.py`:
- Around line 317-378: Tests repeat the same mock_get_devices closure and
monkeypatch.setattr(fomac.Session, "get_devices", ...) in multiple tests;
extract this into a small helper (e.g., _patch_session_devices) that accepts
monkeypatch and a list of MockQDMIDevice and installs a single closure returning
that list via monkeypatch.setattr, then replace the per-test mock_get_devices
definitions and monkeypatch.setattr calls with calls to the new helper in
test_backend_warns_on_unmappable_operation,
test_backend_warns_on_missing_measurement_operation, and
test_backend_validation_uses_inverse_mapping.

In `@test/python/plugins/qiskit/test_sampler.py`:
- Around line 24-33: Extract the DDSIM discovery loop into a shared pytest
fixture (e.g., ddsim_backend) in test/python/plugins/qiskit/conftest.py that
creates and returns a QDMIBackend for the device whose device.name() contains
"DDSIM" or calls pytest.skip("DDSIM device not available") if none found; then
update the sampler fixture in test_sampler.py to accept that ddsim_backend
fixture and return QDMISampler(ddsim_backend) instead of repeating the loop, and
likewise change the estimator fixture (and the similar code in test_backend.py)
to depend on ddsim_backend so all device selection and skip behavior is
centralized.

---

Outside diff comments:
In `@test/python/plugins/qiskit/test_estimator.py`:
- Around line 36-51: The test_estimator_run_simple_observable has a stale
comment about a "mock backend" and only asserts shapes; update it to assert the
actual expected expectation values from the deterministic DDSIM run: remove or
update the comment and add assertions on pub_result.data["evs"] using
pytest.approx to check evs[0] == approx(1.0) and evs[1] == approx(0.0) (and keep
the shape asserts if desired); reference the test function
test_estimator_run_simple_observable, the estimator.run(...) call and the
pub_result.data["evs"] / ["stds"] fields when making the assertions.

In `@test/python/plugins/qiskit/test_sampler.py`:
- Around line 56-58: The comment is stale because the test now runs on a real
DDSIM backend; update the test around bit_array.get_counts() by removing the
"mock backend" phrasing and add an assertion that the observed outcome keys are
only "00" and "11" (e.g., assert set(counts.keys()) <= {"00","11"}) in addition
to the existing total-count check so the test verifies the expected
deterministic Bell-state outcomes; locate this change in the test function using
bit_array.get_counts() and the counts variable.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b068b742-fe63-4d9e-9f37-f941d7d6ec5b

📥 Commits

Reviewing files that changed from the base of the PR and between ea5fd11 and 5e96b16.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (33)
  • CHANGELOG.md
  • UPGRADING.md
  • cmake/AddMQTCoreLibrary.cmake
  • cmake/PackageAddTest.cmake
  • docs/qdmi/driver.md
  • include/mqt-core/qdmi/driver/Driver.hpp
  • json/sc/device.json
  • pyproject.toml
  • python/mqt/core/plugins/qiskit/backend.py
  • src/dd/Simulation.cpp
  • src/ir/QuantumComputation.cpp
  • src/qdmi/devices/dd/CMakeLists.txt
  • src/qdmi/devices/dd/Device.cpp
  • src/qdmi/devices/na/CMakeLists.txt
  • src/qdmi/devices/na/DynDevice.cpp
  • src/qdmi/devices/sc/CMakeLists.txt
  • src/qdmi/devices/sc/DynDevice.cpp
  • src/qdmi/driver/CMakeLists.txt
  • src/qdmi/driver/Driver.cpp
  • test/fomac/CMakeLists.txt
  • test/na/fomac/CMakeLists.txt
  • test/python/plugins/qiskit/conftest.py
  • test/python/plugins/qiskit/test_backend.py
  • test/python/plugins/qiskit/test_converters.py
  • test/python/plugins/qiskit/test_estimator.py
  • test/python/plugins/qiskit/test_mock_backend.py
  • test/python/plugins/qiskit/test_provider.py
  • test/python/plugins/qiskit/test_sampler.py
  • test/qdmi/devices/dd/CMakeLists.txt
  • test/qdmi/devices/na/CMakeLists.txt
  • test/qdmi/devices/sc/CMakeLists.txt
  • test/qdmi/driver/CMakeLists.txt
  • test/qdmi/driver/test_driver.cpp
💤 Files with no reviewable changes (7)
  • test/python/plugins/qiskit/test_provider.py
  • src/qdmi/devices/na/DynDevice.cpp
  • src/qdmi/devices/dd/Device.cpp
  • test/python/plugins/qiskit/conftest.py
  • include/mqt-core/qdmi/driver/Driver.hpp
  • src/qdmi/devices/sc/DynDevice.cpp
  • test/python/plugins/qiskit/test_converters.py

Comment thread src/qdmi/driver/CMakeLists.txt
Comment thread src/qdmi/driver/Driver.cpp
Comment thread src/qdmi/driver/Driver.cpp
Comment thread test/python/plugins/qiskit/test_backend.py
Comment thread test/python/plugins/qiskit/test_estimator.py
Comment thread test/python/plugins/qiskit/test_estimator.py Outdated
Comment thread test/python/plugins/qiskit/test_mock_backend.py
Comment thread test/python/plugins/qiskit/test_sampler.py
burgholzer added 12 commits May 11, 2026 22:22
…ary wrappers

Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
…onverter

Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
…sent

Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
@burgholzer burgholzer force-pushed the smoother-qdmi-integration branch from 5e96b16 to c86bd14 Compare May 11, 2026 20:24
@burgholzer
Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
python/mqt/core/plugins/qiskit/backend.py (2)

230-239: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Normalize operation names before alias remapping/dedup.

Use lowercase once before _QDMI_TO_QISKIT_GATE_MAP lookup. Current case-sensitive lookup can miss aliases and produce duplicate class-gate names with different casing.

Suggested change
-            op_name = op.name()
+            op_name = op.name().lower()
@@
-            if op_name in self._QDMI_TO_QISKIT_GATE_MAP:
-                op_name = self._QDMI_TO_QISKIT_GATE_MAP[op_name]
+            op_name = self._QDMI_TO_QISKIT_GATE_MAP.get(op_name, op_name)

Also applies to: 251-252

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@python/mqt/core/plugins/qiskit/backend.py` around lines 230 - 239, Normalize
op_name to lowercase immediately after retrieving it from op.name() and before
any further checks or the alias lookup so the case-insensitive mapping in
_QDMI_TO_QISKIT_GATE_MAP works and deduping doesn't produce different-cased
duplicates; specifically, update the block that sets op_name = op.name() and the
subsequent lookup against _QDMI_TO_QISKIT_GATE_MAP to use op_name =
op.name().lower(), and apply the same normalization to the later occurrence that
currently performs a lookup/alias remapping (the lines around where op_name is
remapped again) so all comparisons and remaps use the lowercased op_name
consistently.

63-64: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix the gate map types to include instruction classes.

The map stores both instruction instances and classes (e.g., MCXGate, MCPhaseGate from lines 80–82), but type annotations declare only Instruction instances. This will trigger ty diagnostics.

Suggested change
-def _build_gate_mappings_for_backend(
+def _build_gate_mappings_for_backend(
     gate_aliases: dict[str, set[str]],
-) -> tuple[dict[str, set[str]], dict[str, Instruction]]:
+) -> tuple[dict[str, set[str]], dict[str, Instruction | type[Instruction]]]:
@@
-    operation_to_gate: dict[str, Instruction] = {}
+    operation_to_gate: dict[str, Instruction | type[Instruction]] = {}
@@
-    _OPERATION_TO_GATE_MAP: ClassVar[dict[str, Instruction]]
+    _OPERATION_TO_GATE_MAP: ClassVar[dict[str, Instruction | type[Instruction]]]
@@
-    def _map_operation_to_gate(op_name: str) -> Instruction | None:
+    def _map_operation_to_gate(op_name: str) -> Instruction | type[Instruction] | None:

Also applies to lines 86–87, 158–159, and 324–325.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@python/mqt/core/plugins/qiskit/backend.py` around lines 63 - 64, The type
annotations for the Qiskit↔QDMI mappings currently only allow Instruction
instances but the maps also store instruction classes (e.g., MCXGate,
MCPhaseGate); update the annotations to include both classes and instances by
importing typing.Type and typing.Union and changing occurrences like dict[str,
Instruction] or set[Instruction] to dict[str, Union[Instruction,
Type[Instruction]]] or set[Union[Instruction, Type[Instruction]]] (or use
Type[Instruction] where only classes are stored); apply this change to the
function return annotation that currently reads ") -> tuple[dict[str, set[str]],
dict[str, Instruction]]" and to the other map/type hints referenced around the
same module (the maps defined near lines ~86–87, ~158–159, and ~324–325) so all
places that can hold classes or instances accept Type[Instruction] as well as
Instruction.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pyproject.toml`:
- Around line 52-53: The docs dependency group is allowing
qiskit[qasm3-import]>=1.0.0 which mismatches the other groups; update the
`dependency-groups.docs` entry for `qiskit[qasm3-import]` to use the same lower
bound `>=1.1.0` so it matches `project.optional-dependencies.qiskit` and
`dependency-groups.test` (edit the docs group line that currently mentions
`qiskit[qasm3-import]` and change the version specifier to `>=1.1.0`).

In `@test/na/fomac/CMakeLists.txt`:
- Around line 18-35: Change the add_custom_command invocations that copy device
DLLs to use POST_BUILD instead of PRE_BUILD and add per-target existence guards
before referencing each device target; specifically, wrap the copy command for
each device target (MQT::CoreQDMIScDevice, MQT::CoreQDMINaDevice,
MQT::CoreQDMI_DDSIM_Device) in an if(TARGET <device>) ... endif() block and
replace PRE_BUILD with POST_BUILD while keeping the same COMMAND and destination
using $<TARGET_FILE_DIR:${TARGET_NAME}> so the commands only run when the device
targets exist and run after the test target is built.

In `@test/python/plugins/qiskit/test_mock_backend.py`:
- Around line 176-178: The outcome generation incorrectly encodes zero classical
bits because format(0, "00b") yields "0"; update the logic in the block that
computes num_outcomes and outcomes (using self._num_clbits and the outcomes
list) to special-case self._num_clbits == 0 so that outcomes becomes [""] (and
num_outcomes remains 1) instead of producing a phantom "0" bit string; adjust
only the outcome generation around num_outcomes and outcomes to return an
empty-string key for no-measurement programs.

In `@test/qdmi/devices/dd/CMakeLists.txt`:
- Around line 36-43: The custom command that copies the Windows DLL uses
PRE_BUILD which is unreliable across non-Visual-Studio generators; change the
add_custom_command for TARGET ${TARGET_NAME} to use POST_BUILD instead of
PRE_BUILD so the copy runs after the target is built (staging
$<TARGET_FILE:MQT::CoreQDMI_DDSIM_Device> into
$<TARGET_FILE_DIR:${TARGET_NAME}>). Ensure the command still uses the same
TARGET ${TARGET_NAME} and copy_if_different arguments but with POST_BUILD to
avoid timing issues with Ninja and other generators.

In `@test/qdmi/driver/CMakeLists.txt`:
- Around line 19-36: The add_custom_command calls for the test target (TARGET
${TARGET_NAME}) use PRE_BUILD which is generator-dependent; change each
PRE_BUILD to POST_BUILD and make the copy commands depend explicitly on the test
target's build so they run after the target is produced, i.e. update the three
add_custom_command invocations that reference
$<TARGET_FILE:MQT::CoreQDMIScDevice>, $<TARGET_FILE:MQT::CoreQDMINaDevice>, and
$<TARGET_FILE:MQT::CoreQDMI_DDSIM_Device> to use POST_BUILD and ensure the
target dependency is the ${TARGET_NAME} (or add add_dependencies(${TARGET_NAME}
<device-target>) if needed) so the device DLLs are copied deterministically
across generators.

---

Outside diff comments:
In `@python/mqt/core/plugins/qiskit/backend.py`:
- Around line 230-239: Normalize op_name to lowercase immediately after
retrieving it from op.name() and before any further checks or the alias lookup
so the case-insensitive mapping in _QDMI_TO_QISKIT_GATE_MAP works and deduping
doesn't produce different-cased duplicates; specifically, update the block that
sets op_name = op.name() and the subsequent lookup against
_QDMI_TO_QISKIT_GATE_MAP to use op_name = op.name().lower(), and apply the same
normalization to the later occurrence that currently performs a lookup/alias
remapping (the lines around where op_name is remapped again) so all comparisons
and remaps use the lowercased op_name consistently.
- Around line 63-64: The type annotations for the Qiskit↔QDMI mappings currently
only allow Instruction instances but the maps also store instruction classes
(e.g., MCXGate, MCPhaseGate); update the annotations to include both classes and
instances by importing typing.Type and typing.Union and changing occurrences
like dict[str, Instruction] or set[Instruction] to dict[str, Union[Instruction,
Type[Instruction]]] or set[Union[Instruction, Type[Instruction]]] (or use
Type[Instruction] where only classes are stored); apply this change to the
function return annotation that currently reads ") -> tuple[dict[str, set[str]],
dict[str, Instruction]]" and to the other map/type hints referenced around the
same module (the maps defined near lines ~86–87, ~158–159, and ~324–325) so all
places that can hold classes or instances accept Type[Instruction] as well as
Instruction.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4dc3b6e4-f9ad-47f3-9a4c-107a82596cf9

📥 Commits

Reviewing files that changed from the base of the PR and between 5e96b16 and c86bd14.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (33)
  • CHANGELOG.md
  • UPGRADING.md
  • cmake/AddMQTCoreLibrary.cmake
  • cmake/PackageAddTest.cmake
  • docs/qdmi/driver.md
  • include/mqt-core/qdmi/driver/Driver.hpp
  • json/sc/device.json
  • pyproject.toml
  • python/mqt/core/plugins/qiskit/backend.py
  • src/dd/Simulation.cpp
  • src/ir/QuantumComputation.cpp
  • src/qdmi/devices/dd/CMakeLists.txt
  • src/qdmi/devices/dd/Device.cpp
  • src/qdmi/devices/na/CMakeLists.txt
  • src/qdmi/devices/na/DynDevice.cpp
  • src/qdmi/devices/sc/CMakeLists.txt
  • src/qdmi/devices/sc/DynDevice.cpp
  • src/qdmi/driver/CMakeLists.txt
  • src/qdmi/driver/Driver.cpp
  • test/fomac/CMakeLists.txt
  • test/na/fomac/CMakeLists.txt
  • test/python/plugins/qiskit/conftest.py
  • test/python/plugins/qiskit/test_backend.py
  • test/python/plugins/qiskit/test_converters.py
  • test/python/plugins/qiskit/test_estimator.py
  • test/python/plugins/qiskit/test_mock_backend.py
  • test/python/plugins/qiskit/test_provider.py
  • test/python/plugins/qiskit/test_sampler.py
  • test/qdmi/devices/dd/CMakeLists.txt
  • test/qdmi/devices/na/CMakeLists.txt
  • test/qdmi/devices/sc/CMakeLists.txt
  • test/qdmi/driver/CMakeLists.txt
  • test/qdmi/driver/test_driver.cpp
💤 Files with no reviewable changes (7)
  • src/qdmi/devices/dd/Device.cpp
  • test/python/plugins/qiskit/test_provider.py
  • src/qdmi/devices/sc/DynDevice.cpp
  • include/mqt-core/qdmi/driver/Driver.hpp
  • test/python/plugins/qiskit/test_converters.py
  • src/qdmi/devices/na/DynDevice.cpp
  • test/python/plugins/qiskit/conftest.py

Comment thread pyproject.toml
Comment thread test/na/fomac/CMakeLists.txt
Comment thread test/python/plugins/qiskit/test_mock_backend.py
Comment thread test/qdmi/devices/dd/CMakeLists.txt
Comment thread test/qdmi/driver/CMakeLists.txt
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/python/plugins/qiskit/test_mock_backend.py`:
- Line 189: The zip in the dict comprehension that builds the outcome->count
mapping should use strict=True to ensure outcomes and counts_list have the same
length; update the expression that currently uses zip(outcomes, counts_list,
strict=False) to zip(outcomes, counts_list, strict=True) so any future length
mismatches are caught (look for the comprehension referencing outcomes and
counts_list in test_mock_backend.py).
- Around line 174-175: The code returns {} when self._num_clbits == 0 which
breaks the counts-sum invariant; change that branch to return {"": self._shots}
so the single empty-bitstring outcome accumulates all shots (use self._shots and
keep the empty-string key). Also update the zip call that iterates outcomes (the
one pairing sequences of length num_outcomes) to use zip(..., strict=True) to
enforce equal lengths and catch future mismatches; look for the zip(...) using
num_outcomes and replace with zip(..., strict=True).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3b85bc0b-faa2-4ed8-8292-4c0b8c4056ef

📥 Commits

Reviewing files that changed from the base of the PR and between c86bd14 and 07cd0c5.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • pyproject.toml
  • test/python/plugins/qiskit/test_mock_backend.py

Comment thread test/python/plugins/qiskit/test_mock_backend.py Outdated
Comment thread test/python/plugins/qiskit/test_mock_backend.py Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
python/mqt/core/plugins/qiskit/backend.py (1)

61-100: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Update type hints to reflect heterogeneous dict values (instances and class types).

_build_gate_mappings_for_backend stores both Qiskit standard gate instances (from get_standard_gate_name_mapping()) and gate classes (MCXGate, MCPhaseGate) in operation_to_gate and canonical_gates. However, the type signature at line 63 declares the return type as dict[str, Instruction] (instances only), and the matching ClassVar at line 158 and return type of _map_operation_to_gate at line 325 have the same constraint. The variable name gate_instance (line 89) is also misleading since it may bind to a class.

Update to:

  • Line 63: dict[str, Instruction | type[Instruction]]
  • Line 86: operation_to_gate: dict[str, Instruction | type[Instruction]]
  • Line 89: Rename gate_instance to gate for clarity
  • Line 158: _OPERATION_TO_GATE_MAP: ClassVar[dict[str, Instruction | type[Instruction]]]
  • Line 325: _map_operation_to_gate return type to Instruction | type[Instruction] | None

This improves type safety per the coding guideline to prefer fixing typing issues over adding suppressions, especially with [tool.ty.terminal] error-on-warning = true in pyproject.toml.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@python/mqt/core/plugins/qiskit/backend.py` around lines 61 - 100, The return
and variable type hints must allow both Instruction instances and Instruction
classes: change _build_gate_mappings_for_backend's return annotation to
tuple[dict[str, set[str]], dict[str, Instruction | type[Instruction]]], update
the local variable declaration operation_to_gate: dict[str, Instruction |
type[Instruction]], and rename the loop variable gate_instance to gate; also
update the module ClassVar _OPERATION_TO_GATE_MAP to ClassVar[dict[str,
Instruction | type[Instruction]]] and change _map_operation_to_gate's return
type to Instruction | type[Instruction] | None so all signatures reflect
heterogeneous values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/qdmi/driver.md`:
- Line 15: Fix the duplicated word in the qdmi Driver description: replace the
phrase "several devices preloaded devices" with "several preloaded devices" so
the sentence reads e.g. "MQT Core's QDMI Driver, qdmi::Driver, comes with
several preloaded devices that can be used directly." Ensure only the duplicated
word is removed and the class reference qdmi::Driver remains unchanged.

---

Outside diff comments:
In `@python/mqt/core/plugins/qiskit/backend.py`:
- Around line 61-100: The return and variable type hints must allow both
Instruction instances and Instruction classes: change
_build_gate_mappings_for_backend's return annotation to tuple[dict[str,
set[str]], dict[str, Instruction | type[Instruction]]], update the local
variable declaration operation_to_gate: dict[str, Instruction |
type[Instruction]], and rename the loop variable gate_instance to gate; also
update the module ClassVar _OPERATION_TO_GATE_MAP to ClassVar[dict[str,
Instruction | type[Instruction]]] and change _map_operation_to_gate's return
type to Instruction | type[Instruction] | None so all signatures reflect
heterogeneous values.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 54c63590-c05c-4dea-afee-ef0b3ce0e8ca

📥 Commits

Reviewing files that changed from the base of the PR and between 07cd0c5 and 8d76c9a.

📒 Files selected for processing (3)
  • docs/qdmi/driver.md
  • python/mqt/core/plugins/qiskit/backend.py
  • test/python/plugins/qiskit/test_mock_backend.py

Comment thread docs/qdmi/driver.md Outdated
@burgholzer
Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 12, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@burgholzer
Copy link
Copy Markdown
Member Author

burgholzer commented May 12, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 12, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CHANGELOG.md`:
- Line 26: Replace the unhyphenated word "builtin" in the changelog entry "♻️
Build all builtin QDMI devices as shared libraries" with the correct compound
adjective "built-in"; edit the CHANGELOG entry text so it reads "♻️ Build all
built-in QDMI devices as shared libraries" to apply the hyphenation rule for
compound adjectives.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e90d0019-bff4-43d9-a6b8-029f3b41dea2

📥 Commits

Reviewing files that changed from the base of the PR and between 8d76c9a and 6bf85a3.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • docs/qdmi/driver.md
  • python/mqt/core/plugins/qiskit/backend.py

Comment thread CHANGELOG.md
@burgholzer burgholzer merged commit 4fe097c into main May 12, 2026
32 of 33 checks passed
@burgholzer burgholzer deleted the smoother-qdmi-integration branch May 12, 2026 11:49
mergify Bot added a commit that referenced this pull request May 12, 2026
## Description

This PR originally only aimed to address #1352.
However, working on it prompted several follow-ups and improvements for
the QDMI support layer as well as a couple of bugfixes.

The main change here is still that all QDMI devices are now built as
shared libraries by default and that the respective dynamic wrappers
have been removed.
The QDMI Qiskit wrapper now supports multi-controlled gates as exposed
by the DDSIM device.
The QDMI Python tests have been significantly streamlined, and a lot of
the mocking code is replaced with actual device executions.

I will leave this in draft until CI is green because I expect Windows to
cause some problems.
Edit: As expected, Windows is acting up big again.
Edit2: I think I have a working version now. Only took 10h 🫠 

Fixes #1352 

## Checklist



- [x] The pull request only contains commits that are focused and
relevant to this change.
- [x] I have added appropriate tests that cover the new/changed
functionality.
- [x] I have updated the documentation to reflect these changes.
- [x] I have added entries to the changelog for any noteworthy
additions, changes, fixes, or removals.
- [x] I have added migration instructions to the upgrade guide (if
needed).
- [x] The changes follow the project's style guidelines and introduce no
new warnings.
- [x] The changes are fully tested and pass the CI checks.
- [x] I have reviewed my own code changes.

**If PR contains AI-assisted content:**

- [x] I have disclosed the use of AI tools in the PR description as per
our [AI Usage
Guidelines](https://github.com/munich-quantum-toolkit/core/blob/main/docs/ai_usage.md).
- [x] AI-assisted commits include an `Assisted-by: [Model Name] via
[Tool Name]` footer.
- [x] I confirm that I have personally reviewed and understood all
AI-generated content, and accept full responsibility for it.



(cherry picked from commit 4fe097c)

---------

Co-authored-by: Lukas Burgholzer <burgholzer@me.com>
Co-authored-by: Daniel Haag <121057143+denialhaag@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-potential Changes to be backported to the stable branch minor Minor version update QDMI Anything related to QDMI refactor Anything related to code refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

♻️ Refactor QDMI device architecture: Remove static device libraries

2 participants